(Mostly) using NpgsqlBatch and positional parameters behind the scenes #2894
Conversation
|
|
||
| internal class IsArchivedMember: IQueryableMember, IComparableMember, IBooleanMember | ||
| { | ||
| private static readonly string _locator = "d.is_archived"; |
There was a problem hiding this comment.
Should this be a const instead?
There was a problem hiding this comment.
A little bit, as the JIT doesn't need to go through Tier-0 -> Tier-1 for this and then allocate it on the frozen heap.
So for startup time (if that matters) the const is cheaper.
And for consistency with other places where const is used.
There was a problem hiding this comment.
Is the filename a working title?
Should it be CompiledQueryPlan.cs?
There was a problem hiding this comment.
I forgot to move thing around
| NpgsqlParameter ICommandBuilder.AppendParameter(object value) | ||
| { | ||
| _current ??= appendCommand(); | ||
| var name = "p" + _parameterIndex; |
There was a problem hiding this comment.
The native parameter style of Postgres is $i (where i denotes the index), so it's also recommended by Npgsql docs.
This produces a named parameter p1.
Can you change this to being $1 in order to avoid re-parsing the SQL in the Npgsql-layer?
There was a problem hiding this comment.
That's never executed. That's strictly to be able to "plan" the compiled query command.
| private NpgsqlParameter appendParameter(object value, NpgsqlDbType? dbType) | ||
| { | ||
| _current ??= appendCommand(); | ||
| var name = "p" + _parameterIndex; |
There was a problem hiding this comment.
And the same response:)
| NpgsqlParameter[] ICommandBuilder.AppendWithParameters(string text) | ||
| { | ||
| _current ??= appendCommand(); | ||
| var split = text.Split('?'); |
There was a problem hiding this comment.
This is to honor existing customer code?
There was a problem hiding this comment.
It's used quite a bit in the generated code
| return parameters; | ||
| } | ||
|
|
||
| NpgsqlParameter[] ICommandBuilder.AppendWithParameters(string text, char placeholder) |
There was a problem hiding this comment.
This is the same method as above, combine these two with default argument to
| NpgsqlParameter[] ICommandBuilder.AppendWithParameters(string text, char placeholder) | |
| NpgsqlParameter[] ICommandBuilder.AppendWithParameters(string text, char placeholder = '?') |
?
| @@ -1,33 +0,0 @@ | |||
| # Environment Checks | |||
There was a problem hiding this comment.
@jeremydmiller, something went really wrong with docs.
There was a problem hiding this comment.
Getting really bored with how the markdown files get massacred by big changes
| @@ -134,7 +134,7 @@ public void CanPlugInRetryPolicyThatRetriesOnException() | |||
| } | |||
|
|
|||
| // Our retry exception filter should have triggered twice | |||
| Assert.True(m.Count(s => s.IndexOf("relation \"mt_nonexistenttable\" does not exist", StringComparison.OrdinalIgnoreCase) > -1) == 2); | |||
| //Assert.True(m.Count(s => s.IndexOf("relation \"mt_nonexistenttable\" does not exist", StringComparison.OrdinalIgnoreCase) > -1) == 2); | |||
| using Weasel.Core; | ||
| using Weasel.Postgresql; | ||
|
|
||
| namespace Marten.Generated.DocumentStorage |
There was a problem hiding this comment.
Should those generated files be removed?
There was a problem hiding this comment.
It'd speed up the tests if we left them in place
| @@ -54,535 +54,4 @@ This model is comparable to the .Net `IOptions` model. | |||
|
|
|||
| ## Register DocumentStore with AddMarten() | |||
|
|
|||
| ::: tip INFO | |||
There was a problem hiding this comment.
@jeremydmiller this should get back, do you plan to fix it?
| @@ -302,4 +302,4 @@ There is still some discussion on how to leverage this: [Add testing helpers for | |||
| 1. **Parallel Execution**: xUnit runs tests in parallel. If your tests are not isolated, it could lead to unexpected behavior. | |||
| 2. **Database Clean-Up**: You may want to clean up or reset the database state before running each test. Helpers are explained here: [Cleaning up database](/schema/cleaning). | |||
|
|
|||
| Feel free to consult the official documentation for [Alba](https://jasperfx.github.io/alba/), [Wolverine](https://wolverine.netlify.app/), and [xUnit](https://xunit.net/) for more in-depth information. | |||
| Feel fre | |||
There was a problem hiding this comment.
this als should not be removed and all docs above.
No description provided.